Skip to content

Fix issue that caused errors thrown from macro expansion to show up twice in MacroSystem #2115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 6, 2023

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Aug 29, 2023

If the macro expansion of a freestanding expression macro throws an error, expandCodeBlockItem returned nil while adding the thrown error to the macro expansion context. visit(_:CodeBlockItemListSyntax).addResult took the nil return value as an indicator that the macro wasn’t expanded because its macro definition wasn’t found and ended up calling the expansion again in

// Recurse on the child node
newItems.append(visit(node))

Change the return value of expandCodeBlockItem to an enum that indicates whether the macro was not found or if the expansion failed. If the macro was found but the expansion threw an error, we just just retain the macro as-is without calling into visit again.

Fixes #2111
rdar://114592410

@ahoppen
Copy link
Member Author

ahoppen commented Aug 30, 2023

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Aug 31, 2023

@swift-ci Please test Linux

1 similar comment
@ahoppen
Copy link
Member Author

ahoppen commented Aug 31, 2023

@swift-ci Please test Linux

case failure

/// The node that should be expanded was not a macro known to the macro system.
case notAMacro
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unknownMacro

…wice in MacroSystem

If the macro expansion of a freestanding expression macro throws an error, `expandCodeBlockItem` returned `nil` while adding the thrown error to the macro expansion context. `visit(_:CodeBlockItemListSyntax).addResult` took the `nil` return value as an indicator that the macro wasn’t expanded because its macro definition wasn’t found and ended up calling the expansion again in

```swift
// Recurse on the child node
newItems.append(visit(node))
```

Change the return value of `expandCodeBlockItem` to an enum that indicates whether the macro was not found or if the expansion failed. If the macro was found but the expansion threw an error, we just just retain the macro as-is without calling into `visit` again.

Fixes swiftlang#2111
rdar://114592410
@ahoppen ahoppen force-pushed the ahoppen/multiple-errors branch from 2c48690 to c32dca9 Compare September 1, 2023 16:17
@ahoppen
Copy link
Member Author

ahoppen commented Sep 1, 2023

@swift-ci Please test

@ahoppen
Copy link
Member Author

ahoppen commented Sep 1, 2023

@swift-ci Please test Windows

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

assertMacroExpansion reports duplicate diagnostic
2 participants